perf(gfql): re-land #1837 onto master (transformer class built once, 2.5-2.8x compile) - #1842
Merged
Conversation
…parse _build_transformer() defined two @DataClass(frozen=True) helpers and the Lark Transformer subclass INSIDE its own body, so all three types were re-created on every _parse_expr_cached miss. @DataClass generates __init__/__eq__ as source and execs it, so each rebuild ran the compiler. Profiling q7 of the matched graph benchmark (31 cold compiles, 0.649s total) attributed 0.261s -- 40% of ALL compile time -- to this one function: 434 calls producing 868 dataclass creations, 0.120s of it in builtins.exec. The whole LALR(1) parse was 0.151s by comparison. Lowering, not parsing, dominates GFQL compile, and this was most of lowering. The class is now built once per process behind lru_cache(maxsize=1); instances are still created per parse, so nothing stateful is shared and there is no re-entrancy question to argue. Only type creation is hoisted, and a type is a function of the code, not of any query -- so it is classified EXEMPT in the clear-caches enumeration, alongside the Lark parser objects. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YYZRXegrALuXd3NHH5evqx
# Conflicts: # graphistry/tests/compute/gfql/test_clear_caches_covers_every_cache.py
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011AB4RZpph3uSFUpzKnZJcr
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#1837 showed MERGED but its base had been retargeted to
fix/gfql-clear-caches-real-targets(the #1836 stack base), so merge commit 523e128 landed on that branch and is NOT an ancestor of master — caught during the #1841 conflict repair. This PR re-lands exactly that content against master: the transformer-class hoist, its process-singleton registration in the cache registry with a survives-clear pin, and its CHANGELOG Performance entry. Owner already authorized this content's merge (2026-08-02).🤖 Generated with Claude Code
https://claude.ai/code/session_011AB4RZpph3uSFUpzKnZJcr